Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the 'resize' event handler in viewer.js (issue 6158) #6192

Merged
merged 1 commit into from
Jul 10, 2015
Merged

Refactor the 'resize' event handler in viewer.js (issue 6158) #6192

merged 1 commit into from
Jul 10, 2015

Conversation

Snuffleupagus
Copy link
Collaborator

This is the next step towards fixing #6158.

This patch removes the dependency on the state of the scale <select> dropdown from the resize event handler, and instead uses the (in PDFViewer) stored currentScaleValue.
I believe that the way this code is currently written is purely for historical reasons, since originally only the numerical scale was stored internally (hence there was no other way to access the scale value).
However, since we now store the scale value, we should use it instead of quering the DOM. This helps ensure that the internally stored scale value is always accurately displayed in the UI (which should be good since, after the creation of PDFViewer, the <select> DOM element is now updated by an event handler).

@timvandermeij Since you've been kind enough to review the previous patches in this series, could you please review this one as well?

*This is the next step towards fixing 6158.*

This patch removes the dependency on the state of the scale `<select>` dropdown from the `resize` event handler, and instead uses the (in `PDFViewer`) stored `currentScaleValue`.
I believe that the way this code is currently written is purely for historical reasons, since originally *only* the numerical scale was stored internally (hence there was no other way to access the scale value).
However, since we now store the scale value, we should use it instead of quering the DOM. This helps ensure that the internally stored scale value is always accurately displayed in the UI (which should be good since, after the creation of `PDFViewer`, the `<select>` DOM element is now updated by an event handler).
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/da87e845c8a63ca/output.txt

timvandermeij added a commit that referenced this pull request Jul 10, 2015
Refactor the 'resize' event handler in viewer.js (issue 6158)
@timvandermeij timvandermeij merged commit c9b6b69 into mozilla:master Jul 10, 2015
@timvandermeij
Copy link
Contributor

Looks much better than before, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants